-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
psp-8771 & psp-8683 | Lease expiry using renewal information #4189
Conversation
…y definition for leases
✅ No secrets were detected in the code. |
✅ No secrets were detected in the code. |
{ | ||
dest.LFileNo = src.lease.LFileNo; | ||
dest.MotiRegion = src.lease.RegionCodeNavigation?.RegionName; | ||
dest.StartDate = src.lease.OrigStartDate.FilterSqlMinDate().ToNullableDateOnly(); | ||
dest.EndDate = src.lease.OrigExpiryDate?.FilterSqlMinDate().ToNullableDateOnly(); | ||
dest.EndDate = src.expiryDate.ToNullableDateOnly(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to filter the sql min date here? previously this would cause exceptions converting from an sql min date to a date time.
@@ -36,7 +36,7 @@ private static void MapLease((Entity.PimsLeasePeriod period, Entity.PimsLease le | |||
dest.InspectionNotes = src.lease.InspectionNotes; | |||
dest.InspectionDate = src.lease.InspectionDate?.FilterSqlMinDate(); | |||
dest.LeaseNotes = src.lease.LeaseNotes; | |||
dest.IsExpired = (src.lease.GetExpiryDate() < DateTime.Now).BoolToYesNo(); | |||
dest.IsExpired = (src.expiryDate < DateTime.Now).BoolToYesNo(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to include renewals in this calculation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100%. I dont think this field is actually used in the frontend. But I will clean it up as tech debt
const excercisedRenewalDates = renewals.filter(r => r.isExercised).flatMap(fr => fr.expiryDt); | ||
|
||
let calculatedExpiry: string | null = lease?.expiryDate ?? ''; | ||
for (let i = 0; i < excercisedRenewalDates.length; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also do with a sort? this seems fine though.
@FuriousLlama here is the only area I'm seeing a mismatch so far: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved, only found one instance in application where no expiry not working as intended.
✅ No secrets were detected in the code. |
Updated lease expiry search to use renewal information. Updated expiry definition for leases